Rtnetlink version Upgrade #30
Conversation
- upgrade rtnetlink to latest version: 0.4.1.
Still unstable
- upgrade netlink-packet-route to version 0.19.
- Introduce netlink-packet-utils. Some of the
items we were previously using in
netlink-packet-route are now only accessible
via that library.
Majority of the changes have been made in
the `netlink.rs` files that exist both in
`holo-routing` and `holo-interface`.
There have been several changes on rtnetlink &
netlink-packet-route and key files, functions and
structs have also been moved in the library.
The changes have been reflected in this commit.
Signed-off-by: Paul Wekesa <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## vrrp #30 +/- ##
==========================================
- Coverage 63.50% 63.42% -0.09%
==========================================
Files 177 177
Lines 29643 29660 +17
==========================================
- Hits 18825 18812 -13
- Misses 10818 10848 +30 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
@Paul-weqe Thanks for this! Looks great overall, just have a couple of minor comments.
holo-interface/src/netlink.rs
Outdated
| use netlink_sys::{AsyncSocket, SocketAddr}; | ||
| use rtnetlink::{new_connection, Handle}; | ||
| use tracing::{error, trace}; | ||
|
|
||
| use crate::interface::Owner; | ||
| use crate::Master; | ||
|
|
||
| const AF_INET: u16 = libc::AF_INET as u16; | ||
| const AF_INET6: u16 = libc::AF_INET6 as u16; |
There was a problem hiding this comment.
nit: It might be cleaner to import the constants (use libc::{AF_INET, AF_INET6};) rather than redefining them.
There was a problem hiding this comment.
@rwestphal libc::AF_INET is of type i32. I did this for to be type u16 so that we can use it inside the parse_address method.
holo-interface/src/netlink.rs
Outdated
| for flg in &msg.header.flags { | ||
| flgs += u32::from(*flg); | ||
| } | ||
| if flgs & u32::from(LinkFlag::Running) != 0 { |
There was a problem hiding this comment.
Couldn't we use msg.header.flags.contains(LinkFlag::Running) instead?
remove the AF_INET const definitions on top of `holo-interface`'s netlink.rs. change procedure of looking for flags to `msg.header.flags.contains` Signed-off-by: Paul Wekesa <[email protected]>
|
@rwestphal your help is requested. Linter is failing despite the line it says is failing does not exist on the branch ( Can we treat that as a false negative ? |
There was a problem hiding this comment.
@rwestphal your help is requested.
Linter is failing despite the line it says is failing does not exist on the branch (
upgrade-rtnetlink). The line exists invrrpbranch where the change has also been made but it still fails.Can we treat that as a false negative ?
Yeah that lint error doesn't make sense.
Thanks for the updates! I'll go ahead and merge this.
Would you mind opening the same PR against master as well? The more we can move to master, the less likely we'll run into conflicts when merging the vrrp branch later.
|
@rwestphal opening in a few. |
Majority of the changes have been made in
the
netlink.rsfiles that exist both inholo-routingandholo-interface.There have been several changes on rtnetlink &
netlink-packet-route and key files, functions and
structs have also been moved in the library.
The changes have been reflected in this commit.
The upgrade has been necessitated due to the need for
the ability to edit mac addresses while creating an interface
(on the
vrrpbranch)